-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GR-41675] Only require libmanagement_ext if it's actually needed. #5122
[GR-41675] Only require libmanagement_ext if it's actually needed. #5122
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although I don't really understand why we need to add management_ext
twice in the first place.
...vm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JNIRegistrationManagementExt.java
Show resolved
Hide resolved
1024220
to
8522603
Compare
...vm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JNIRegistrationManagementExt.java
Outdated
Show resolved
Hide resolved
On some systems/apps, libawt_headless gets pulled in. This currently brings in libmanagement_ext unconditionally. However, libmanagement_ext should only be present for linking iff com.sun.management.internal.OperatingSystemImpl class becomes reachable. Closes: oracle#5119
8522603
to
84c9266
Compare
Thanks for the reviews! |
If somebody could help with getting it integrated, I'd appreciate it. |
@fniephaus Could you please help getting this fix integrated? Thank you! |
Sure, but I'm afraid this won't make it into 22.3. |
That's fine. Thanks! |
You got it! |
Internal PR is at #5195. |
On some systems/apps, libawt_headless gets pulled in. This currently brings in libmanagement_ext unconditionally. However, libmanagement_ext should only be present for linking iff
com.sun.management.internal.OperatingSystemImpl class becomes reachable. The fix is to only add
management_ext
as a dependency forawt_headless
when classOperatingSystemImpl
is actually reachable.Closes: #5119